Similar test as above (test_parquet_chunked_reader) but with a struct table.

^Here we adjust the num_rows for the last page when input_pass_limit > 0 to compensate for lists. This leads to a false negative is_bounds_page for the last page of str cols due to the page_end > end predicate. This false negative results in page_bounds() including total size of last page instead of clipping it at nrows. This finally manifests in the out_buf.create_string_data() with larger num_bytes than data size leading to the extra \x00 chars in the column.

@nvdbaranec I didn't see the effect of adjusting num_rows for the last page showing up for other col types and is_bounds_page is only used in string decoders so I am hoping other decoders don't rely on a similar predicate (at least doesn't seem like in the tests).

You know it's a convoluted bug when the fix is 99% comment.

Should we try to keep the condition and change thenum_rows adjustment? This fix feels like a workaround for incorrect num_rows (expecting to be wrong on this one, but want to make sure we at least consider this).

Ikr, I have been debating myself with this one as well. I am not fully sure if we would break things by adjusting num_rows for the last pages of cols (to what?) in generate_list_column_row_count_estimates() and keep the original condition here. Maybe @nvdbaranec or @etseidl can comment here better.

For now, I chose to relax this condition as this function (is_bounds_page()) is only being used in string decoders (narrower scope) and the described problem is only being seen in string cols.

This makes me nervous. So in the normal case, won't this mean every final page is a bounds page, regardless of whether we're using num_rows? That means unnecessary processing when all we want to do is read the entire page anyway. Could we add a boolean argument to is_bounds_page if num_rows has been modified, or add a field to the PageInfo struct? Or maybe do as @vuule suggests (change the num_rows adjustment)...perhaps add an adjusted_num_rows 🤷

Thanks for the feedback, Ed! I have added a new boolean field to PageInfo which tells if the num_rows for this page has been adjusted and if so, we relax the condition.

You know it's a convoluted bug when the fix is 99% comment.

Should we try to keep the condition and change thenum_rows adjustment? This fix feels like a workaround for incorrect num_rows (expecting to be wrong on this one, but want to make sure we at least consider this).